Merged
Conversation
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
commented
Feb 19, 2026
tests/test_vws.py
Outdated
| expected_prefix: bytes, | ||
| ) -> None: | ||
| """The returned bytes match the requested format.""" | ||
| target_id = vws_client.add_target( |
Member
Author
There was a problem hiding this comment.
This isn't right. We should be (here and elsewhere) working with a VuMark target, not a cloud target.
Member
Author
There was a problem hiding this comment.
See VWS-Python/vws-python-mock#2962 for enabling this.
Member
Author
There was a problem hiding this comment.
See VWS-Python/vws-python-mock#2961 for getting the right error with this test.
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
adamtheturtle
commented
Feb 20, 2026
src/vws/vumark_service.py
Outdated
| UnknownTargetError, | ||
| ) | ||
| from vws.vumark_accept import VuMarkAccept | ||
| from vws.vws import _target_api_request |
Member
Author
There was a problem hiding this comment.
This is importing a private function - not good. Make it a public function in a private shared helper file.
adamtheturtle
commented
Feb 20, 2026
| @@ -20,6 +20,7 @@ | |||
| from vws.exceptions.vws_exceptions import ( | |||
Member
Author
There was a problem hiding this comment.
Can the changes here be reverted?
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| # Every HTTP path which can raise this error has the target ID as the | ||
| # second path segment, e.g. `/something/{target_id}` or | ||
| # `/something/{target_id}/more`. | ||
| return path.split(sep="/")[2] |
There was a problem hiding this comment.
Brittle target ID parsing from URLs
Medium Severity
The target_id properties now return path.split(\"/\")[2], which can raise IndexError or return the wrong segment if the URL path has a prefix (e.g., a base URL with a path like /v1) or an unexpected shape. This can break error reporting for UnknownTargetError and related exceptions.
Additional Locations (2)
| target_id: str, | ||
| instance_id: str, | ||
| accept: VuMarkAccept, | ||
| ) -> bytes: |
There was a problem hiding this comment.
No default accept format despite stated behavior
Low Severity
VuMarkService.generate_vumark_instance requires accept: VuMarkAccept with no default, but the PR description/test plan claims “default PNG format behavior.” If callers rely on a default, the new API won’t match the intended contract and will error at call time.
| See https://github.com/VWS-Python/vws-python-mock/issues/2961 for | ||
| writing this test. | ||
| """ | ||
|
|
There was a problem hiding this comment.
Placeholder test silently passes without assertions
Low Severity
test_invalid_target_type contains only a docstring and no assertions or execution. It will always pass, which can give a false sense of coverage for InvalidTargetTypeError behavior.
Implement VuMark instance generation API with support for multiple image formats (PNG, SVG, PDF). Add InvalidAcceptHeaderError and InvalidInstanceIdError exceptions for proper error handling. Includes comprehensive tests for all VuMark formats. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Add BadRequestError exception for the BadRequest result code returned by the VuMark endpoint when invalid JSON is sent. Map it in generate_vumark_instance and include it in the exception inheritance test. Bump vws-python-mock to 2026.2.18.2 which adds full VuMark auth endpoint testing support. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Fix pylint wrong-spelling-in-comment (C0401) for the word 'enum' in the VuMark exception comments. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Check magic bytes/prefix for each format (PNG, SVG, PDF) rather than just asserting non-empty bytes are returned. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
They are now referenced explicitly in the parametrized test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix Sphinx spell checker failures for the result code names used in the new exception docstrings. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace single-quoted result code names with double backticks so the Sphinx spell checker treats them as inline code rather than words. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds extra_headers parameter to _target_api_request and content field to Response, allowing generate_vumark_instance to reuse shared auth/signing logic instead of duplicating it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Centralises TooManyRequestsError and ServerError raising so callers (make_request, generate_vumark_instance) no longer duplicate the logic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds extra_headers and optional expected_result_code (None = binary/HTTP-200 success) to make_request, and adds VuMark-specific result codes to its error dispatch dict, so generate_vumark_instance no longer needs to call _target_api_request directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Now that generate_vumark_instance uses make_request, _target_api_request can be a pure sign-and-send function again. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix target_id property extraction for /targets/{id}/instances URL pattern
- Add InvalidTargetTypeError exception
- Add vumark_vws_client and vumark_target_id fixtures using a pre-populated
VuMark template target (requires vws-python-mock#2962)
- Update tests to use VuMark target instead of a standard cloud target
- Add test_invalid_target_type test (requires vws-python-mock#2961)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
VuMark generation targets a different database type (VuMark vs Cloud Reco), so it belongs in its own class rather than VWS, mirroring how CloudRecoService is separate from VWS. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add 'vumark' to spelling dict - Remove is_vumark_template from Target() until mock#2962 is implemented - Make test_invalid_target_type a placeholder until mock#2961 is implemented - Rephrase fixture docstrings to avoid spelling check on 'pre' Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes Pyright reportPrivateUsage error: _target_api_request was defined in vws.py but used in vumark_service.py. Move it to a shared private module with a public name so both can import it cleanly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mypy flags VuMarkTarget as not explicitly exported from mock_vws.database; import it from mock_vws.target instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
adamtheturtle
force-pushed
the
adamtheturtle/add-vumark-support
branch
from
6e4cba9 to
080e79f
Compare
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
No VWS method passes expected_result_code=None; VuMarkService handles binary responses via its own direct target_api_request call. Removing the unused branch restores 100% test coverage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement VuMark instance generation API with support for multiple image formats (PNG, SVG, PDF). Add InvalidAcceptHeaderError and InvalidInstanceIdError exceptions for proper error handling.
Changes
Test Plan
🤖 Generated with Claude Code
Note
Medium Risk
Adds a new API surface and refactors the shared authenticated request path (
VWS.make_request) plus response shape, which could subtly affect existing callers’ behavior and error handling.Overview
Adds VuMark instance generation support via a new
VuMarkService.generate_vumark_instance()API, plus aVuMarkAcceptenum for requesting PNG/SVG/PDF outputs.Refactors authenticated Target API calls into a shared
target_api_requesthelper (now used byVWS.make_request), extendsResponseto retain rawcontent, and expands error handling with new VuMark-related exceptions (andBadRequest) plus more robusttarget_idextraction for relevant errors. Documentation and tests are updated to cover the new service, formats, andInvalidInstanceIdbehavior.Written by Cursor Bugbot for commit 90c00a1. This will update automatically on new commits. Configure here.